Skip to content

[Feature] Extend selector error message#875

Draft
jan-janssen wants to merge 5 commits into
mainfrom
selector_error
Draft

[Feature] Extend selector error message#875
jan-janssen wants to merge 5 commits into
mainfrom
selector_error

Conversation

@jan-janssen

@jan-janssen jan-janssen commented Dec 30, 2025 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling when selecting values from returned data: invalid or non-existent selectors now produce clearer error messages to help identify mismatches between requested selectors and actual results. The behavior for empty/null results remains unchanged.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Dec 30, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 65a2c7a0-b9d4-492b-ae2b-bef90a4327b7

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

FutureSelector.result now guards access to result[self._selector]: it returns the indexed value only when the selector is an int and result is a tuple/list, or when the selector exists as a key in the result; otherwise it raises a detailed KeyError. None results still return None.

Changes

Cohort / File(s) Summary
FutureSelector change
src/executorlib/standalone/select.py		 Replace unconditional indexing with guarded access: allow integer indexing into sequences or keyed lookup into mappings; when lookup fails, raise a KeyError with selector and result-type details; preserve None handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 I peeked inside a result with care,
"Are you a list, tuple, or keyed affair?"
If keys are missing, I bark a KeyError drum,
Else out pops the value — hop, here we come! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: extending the error message for selector operations by adding guarded access and detailed error reporting.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch selector_error

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot reviewed Dec 30, 2025
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3281a55 and e2bba51.

📒 Files selected for processing (1)
  • src/executorlib/standalone/select.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/executorlib/standalone/select.py (1)
src/executorlib/task_scheduler/file/shared.py (1)
  • result (24-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: unittest_slurm_mpich
  • GitHub Check: benchmark (ubuntu-latest, 3.13, .ci_support/environment-mpich.yml)
  • GitHub Check: unittest_win
  • GitHub Check: benchmark (ubuntu-latest, 3.13, .ci_support/environment-openmpi.yml)

Comment thread src/executorlib/standalone/select.py Outdated
@codecov

codecov Bot commented Dec 30, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.20%. Comparing base (1ec85bf) to head (60e6825).

Files with missing lines Patch % Lines
src/executorlib/standalone/select.py 66.66% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #875      +/-   ##
==========================================
- Coverage   94.24%   94.20%   -0.05%     
==========================================
  Files          39       39              
  Lines        2119     2121       +2     
==========================================
+ Hits         1997     1998       +1     
- Misses        122      123       +1

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jan-janssen jan-janssen marked this pull request as draft December 30, 2025 10:12
@jan-janssen jan-janssen changed the title Feature: Extend selector error message [Feature] Extend selector error message Jan 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jan-janssen